-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Get baseline 100% clean coverage #1553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fbd18c1 to
0972fbb
Compare
05f7591 to
338eb53
Compare
d148a88 to
9577775
Compare
9577775 to
c5ef006
Compare
Kinda cross but there doesn't seem to be a clean way around it
I hate this, I wish we didn't need to do this. I'll fix them later, they're now my mortal enemy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer for this to be called test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
src/mcp/cli/claude.py
Outdated
| try: | ||
| config_file.write_text("{}") | ||
| except Exception: | ||
| except Exception: # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one above covers this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
src/mcp/client/auth/oauth2.py
Outdated
| async def get_tokens(self) -> OAuthToken | None: | ||
| """Get stored tokens.""" | ||
| ... | ||
| ... # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can add ... on the ignore rule in the pyproject.toml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
pyproject.toml
Outdated
| exclude_also = [ | ||
| '\A(?s:.*# pragma: exclude file.*)\Z' | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if we avoid this, it seems too easy...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Kludex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Adds commit 89e9c43 to .git-blame-ignore-revs to exclude the large-scale addition of # pragma: no cover comments from git blame output. This commit established a 100% code coverage baseline by adding pragma comments throughout the codebase. While important for coverage tracking, these annotations are noise in git blame and should be ignored to show the original authors of substantive code changes. Github-Issue: #1553
Motivation and Context
This adds 100% coverage by adding
# pragma: no coverin a bunch of places.This is taking over the work from #1354
Currently this PR is set to draft as it's the initial run of doing this for the whole repo. Need to get some eyes on this to make sure we're doing this right.
TODO:
# pragma: no cover's added where they shouldn't be, like if the line is actually covered I would want these removed.How Has This Been Tested?
Breaking Changes
Hopefully nothing
Types of changes
Checklist
Additional context